Make toStrictEqual and strict Bun.deepEquals distinguish null-prototype objects from object literals by robobun · Pull Request #37776 · oven-sh/bun

robobun · 2026-08-12T06:49:11Z

Problem

On Bun 1.4.0, expect(Object.assign(Object.create(null), { a: 1 })).toStrictEqual({ a: 1 }) passes; jest and vitest fail it. Same for an Object.groupBy result against a literal, and Bun.deepEquals(Object.create(null), {}, true) returns true.
Strict mode's "same type" check compares class names, and a null-prototype object and an object literal both report "Object", so strict mode cannot tell them apart. Class instances are already caught by that check.
node:assert.deepStrictEqual and util.isDeepStrictEqual already fail these since node compat batch: callback-throw dispatch, Assert class + native deep-equality parity, Intl gate + URL/buffer fallout, compile cache, watch kill-signal, profilers (+98 tests) #34660; this is the bun:test / Bun.deepEquals side of the same gap. toEqual passes them in every runner and keeps doing so.

Fix

In strict mode, after the class names match, read both prototypes and report unequal when exactly one of them is null.
Only the strict type check gets tighter. Loose mode, toMatchObject, the skipPrototype variants, and the node entry point (which already compared prototypes by identity before this point) behave as before, and two null-prototype objects still compare equal.
One divergence from jest is left alone: an object whose chain ends in a null-prototype object further up (Object.create(Object.create(null))) still equals {} here.
Verification: the new toStrictEqual test and the three new strict Bun.deepEquals tests fail on Bun 1.4.0 and pass with this change; the expect test also passes under vitest 4.1.9. Docs gain the null-prototype example.

Background

toStrictEqual and Bun.deepEquals(a, b, true) share one C++ deep-equality routine, specialised by template flags. The same routine backs node:assert.deepStrictEqual (prototype identity checked) and node's skipPrototype mode (every prototype check off).
Strict mode's type check uses JSC's calculatedClassName(), which names an object after its constructor and falls back to "Object" when there is none, so {} and Object.create(null) look identical to it.
jest and vitest instead compare a.constructor === b.constructor in toStrictEqual; a null-prototype object has no constructor, so it never strictly equals a literal there. Arrays are exempt in both jest and this routine.
For a Proxy, reading the prototype invokes its getPrototypeOf trap, so the trap's answer decides, not the target's prototype.
Null-prototype objects are common return values: Object.groupBy, querystring.parse, parseArgs().values, and { __proto__: null } literals.

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/bun-object/deep-equals.test.ts

Original description

Repro

import { expect, test } from "bun:test";

test("null-prototype object vs object literal", () => {
  expect(Object.assign(Object.create(null), { a: 1 })).toStrictEqual({ a: 1 }); // passes in bun, fails in jest and vitest
  expect({ a: 1 }).toStrictEqual({ __proto__: null, a: 1 }); // same
  expect(Object.groupBy([1, 2], n => (n % 2 ? "odd" : "even"))).toStrictEqual({ odd: [1], even: [2] }); // same
});

Bun.deepEquals(Object.create(null), {}, true); // true, documented strict mode says instances and literals differ

Verified on Bun 1.4.0. toEqual passes all of these in every runner, and keeps doing so.

Cause

toStrictEqual and Bun.deepEquals(a, b, true) go through Bun__deepEquals<isStrict = true> in src/jsc/bindings/bindings.cpp. The strict "same type" check compares JSObject::calculatedClassName() of both operands. That returns "Object" both for an object inheriting from Object.prototype and for an object with no prototype at all (the constructor lookup finds nothing and it falls back to the class info name), so a null-prototype object and an object literal pass as the same type. Class instances were already caught by this check; null-prototype objects are the one shape it cannot see.

Jest and vitest fail these: toStrictEqual runs the typeEquality tester, which compares a.constructor === b.constructor, and a null-prototype object has no constructor (undefined !== Object). Checked against vitest 4.1.9's equals() with the toStrictEqual tester list:

Object.create(null){a:1} vs {a:1}             toStrictEqual=false  toEqual=true
{a:1} vs Object.create(null){a:1}             toStrictEqual=false  toEqual=true
nested: {x:nullproto} vs {x:{a:1}}            toStrictEqual=false  toEqual=true
array element: [nullproto] vs [{a:1}]         toStrictEqual=false  toEqual=true
Object.groupBy result vs literal              toStrictEqual=false  toEqual=true
both null-proto                               toStrictEqual=true   toEqual=true

node:assert.deepStrictEqual / util.isDeepStrictEqual already fail these since #34660 (they compare prototypes by identity on their own entry point); this is the bun:test / Bun.deepEquals side of the same gap. Bun's own util.inspect also already treats the two shapes as different types ([Object: null prototype] {} vs {}), and the repo's parseArgs tests write { __proto__: null } on the expected side of toStrictEqual precisely because they expect this to be checked.

Fix

In the isStrict && !skipPrototypeIdentity block, after the class names match, read both [[Prototype]]s and return false when exactly one of them is null. This is the fixing hunk:

if constexpr (!checkPrototypes) {
    JSValue proto1 = o1->getPrototype(globalObject);
    RETURN_IF_EXCEPTION(scope, false);
    JSValue proto2 = o2->getPrototype(globalObject);
    RETURN_IF_EXCEPTION(scope, false);
    if (proto1.isNull() != proto2.isNull())
        return false;
}

Why this shape:

It only tightens the strict type check, which is the step that lost the information. Loose mode (toEqual, Bun.deepEquals(a, b)), asymmetric matchers, toMatchObject and the skipPrototype variants (Bun.deepEquals(a, b, true, true), util.isDeepStrictEqual(a, b, true)) are untouched. The node entry point (checkPrototypes) already compared the prototypes by identity before reaching this block, so the new lookups are compiled out there and it keeps invoking a Proxy's getPrototypeOf trap once per side, as before.
It stays name based rather than switching to jest's constructor identity, so objects from another realm (node:vm) still compare the way they did. Null vs non-null is the one distinction calculatedClassName collapses that shows up in practice (Object.create(null), { __proto__: null }, Object.groupBy, querystring.parse, parseArgs().values, node style fs.promises results).
getPrototype(globalObject) is the same call the node entry point uses a few lines up (and the same thing util.inspect consults when it prints [Object: null prototype]). For ordinary objects it is a type-info bit test plus a load. For a Proxy it is the getPrototypeOf trap result, which only matters Proxy vs Proxy, since a Proxy already fails the class name check against a non-Proxy. That coincides with jest unless a trap contradicts its target (jest reads .constructor through the get trap instead); the tests pin the trap-result behaviour so the choice is explicit. Both new calls have RETURN_IF_EXCEPTION; the file was also run under BUN_JSC_validateExceptionChecks=1.
Arrays are not affected (they take the array branch above this check), matching jest, which exempts arrays from typeEquality.

The surrounding comments that described Bun.deepEquals/expect() as prototype-blind are updated, and the null-prototype case is added to the strict mode list in the Bun.deepEquals docs next to the class instance example.

One remaining divergence, left alone as it does not come up in practice: an object whose prototype chain ends in a null-prototype object without ever defining constructor (Object.create(Object.create(null))) still compares equal to {} here, while jest rejects it.

Tests

test/js/bun/test/expect.test.js: new toStrictEqual() test covering both operand orders, nesting in an object property and an array element, two null-prototype objects still being strictly equal (and still differing on values / undefined properties), Object.groupBy, and toEqual continuing to pass for all of it. This file also runs under jest and vitest; the new test passes under vitest 4.1.9.
test/js/bun/bun-object/deep-equals.test.ts: the existing it.failing for Bun.deepEquals(Object.create(null), {}, true) now passes and is un-marked and extended; new tests for two null-prototype objects, for Proxies (a Proxy around a null-prototype target, and traps that report a different prototype than their target, which is what distinguishes reading the trap from unwrapping the target), and a guard that util.isDeepStrictEqual(a, b, true) (node's skipPrototype mode, which instantiates the same function with skipPrototypeIdentity) still treats a null-prototype object and a literal as equal, as node does.

On Bun 1.4.0 exactly the new toStrictEqual test and the three new Bun.deepEquals strict mode tests fail; with this change both files pass (416 and 49 tests). Also ran test/js/node/assert/ (406 pass), test/js/node/util/parse_args/ (all toStrictEqual uses there already spell out __proto__: null; the only failure is a pre-existing 1000x Bun.gc() stress test timing out on the debug build) and the other test files in the repo that use toStrictEqual; the only failures there were debug build timeouts and container networking limits, none involving equality, so nothing else relied on the old behaviour.

#32872 contains this same check as one of four unrelated fixes, but it predates #34660 and no longer applies to main (conflicts in bindings.cpp and two test files), so this is the null-prototype part on its own.

…pEquals The strict "same type" check in Bun__deepEquals compares JSObject::calculatedClassName, which reports "Object" both for an object inheriting from Object.prototype and for one with no prototype at all, so expect(Object.create(null)).toStrictEqual({}) and Bun.deepEquals(Object.create(null), {}, true) passed. Jest and vitest fail them: toStrictEqual's typeEquality compares constructors, and a null-prototype object has none. After the class names match, also reject the pair when exactly one side has a null [[Prototype]]. Loose mode (toEqual, Bun.deepEquals without strict) and the skipPrototype variants are unchanged; node:assert already compared prototypes by identity.

coderabbitai · 2026-08-12T06:49:20Z

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 83c4c404-571f-4a35-b7e9-ab1e74d06e0e

📥 Commits

Reviewing files that changed from the base of the PR and between 626034f and 89c10d1.

📒 Files selected for processing (5)

docs/guides/util/deep-equals.mdx
docs/runtime/utils.mdx
src/jsc/bindings/bindings.cpp
test/js/bun/bun-object/deep-equals.test.ts
test/js/bun/test/expect.test.js

_{Comment @coderabbitai help to get the list of available commands.}

robobun · 2026-08-12T06:49:59Z

Reproduced on Bun 1.4.0 with USE_SYSTEM_BUN=1 bun test test/js/bun/test/expect.test.js (the new toStrictEqual() null-prototype test fails) and test/js/bun/bun-object/deep-equals.test.ts (the three new strict mode tests fail); both files pass with this branch. Reference semantics checked against vitest 4.1.9, see the PR description.

CI for the current head (89c10d1): every job that ran passed (192/192). The build is marked failed only because the two darwin 26 aarch64 - test-bun jobs expired without ever getting an agent; that lane is currently expiring on every build in the pipeline regardless of branch, so a re-run would not change anything here. The remaining annotations are retries that passed and are unrelated to equality (fs read stream, inspect-error-leak, child_process, ctrl-c, grpc).

Fix: #37776 (this PR), ready for review.

…compared prototypes by identity

robobun · 2026-08-12T07:08:05Z

^{Updated 11:05 AM PT - Aug 12th, 2026}

❌ @robobun, your commit 89c10d1 has some failures in Build #93178 (All Failures)

🧪 To try this PR locally:

bunx bun-pr 37776

That installs a local version of the PR into your bun-37776 executable, so you can run:

bun-37776 --bun

claude

I re-reviewed after 9c27643 and found no issues — the if constexpr (!checkPrototypes) guard addresses the earlier note about redundant getPrototype calls on the node entry point. Since this tightens what toStrictEqual / strict Bun.deepEquals accept for every user test suite, a human sign-off on the semantic change is still worthwhile.

Also checked: exception handling on both new getPrototype calls (RETURN_IF_EXCEPTION present, throwing-Proxy test covers both operand orders); loose mode, skipPrototypeIdentity, and the array fast path stay untouched; the un-marked it.failing plus new tests cover nesting, both operand orders, and two-null-proto equality.

Extended reasoning...

Overview

The PR adds a null-prototype check to Bun__deepEquals in src/jsc/bindings/bindings.cpp so that strict-mode Bun.deepEquals and expect().toStrictEqual() distinguish Object.create(null) / { __proto__: null } from ordinary object literals, matching jest and vitest. The change is ~12 lines of C++ plus comment updates, two doc additions, a converted it.failing test in deep-equals.test.ts with new Proxy/throwing-trap coverage, and a new toStrictEqual test block in expect.test.js.

Since my previous review, commit 9c27643 wrapped the new check in if constexpr (!checkPrototypes), so the node:assert/node:util entry point (which already compares prototypes by identity earlier in the function) no longer re-invokes getPrototype — that fully resolves the nit I raised.

Security risks

None. The change reads [[Prototype]] via the existing getPrototype(globalObject) helper (same call used a few lines above for the node path), with RETURN_IF_EXCEPTION after each call so a throwing Proxy trap propagates rather than being swallowed. No new allocation, no untrusted-size arithmetic, no user-controlled indexing.

Level of scrutiny

High. Bun__deepEquals backs every toEqual/toStrictEqual assertion in every bun:test suite and is on the hot path for object comparison. This is also a user-visible behavior change: tests that previously passed expect(Object.groupBy(...)).toStrictEqual({...}) will now fail (correctly, per jest/vitest). The implementation itself is small and straightforward, but the semantic decision — tightening a matcher that users depend on — warrants a maintainer's sign-off rather than an automated approval.

Other factors

Tests are thorough: both operand orders, nested in objects and arrays, two null-proto objects still equal, Object.groupBy, Proxy with and without a throwing getPrototypeOf trap, and toEqual staying unchanged. The PR author verified against vitest 4.1.9 and ran the repo's other toStrictEqual consumers.
The two comment-cop bot flags on lines 819 and 962 look like heuristic false positives to me: line 819 is a pre-existing 3-line comment that this PR only reworded by two words, and line 962 is a 3-line note explaining why calculatedClassName alone is insufficient and why the node path is excluded — both explain non-obvious semantics rather than justify a workaround. A maintainer can decide whether to trim them.
No bugs were found by the bug-hunting system on this revision.

claude

LGTM — the earlier feedback (compiling the null-prototype check out on the checkPrototypes node path) is addressed, and the comment-cop notes were resolved by collapsing to one-liners.

What was reviewed:

Traced all six deepEqualsWrapperImpl instantiations: the new check runs only on strictDeepEquals/jestStrictDeepEquals (isStrict && !skipPrototypeIdentity && !checkPrototypes); loose mode, toMatchObject, and both node entry points compile it out.
getPrototype(globalObject) has RETURN_IF_EXCEPTION after each call, and the throwing-trap test covers both operand orders.
Confirmed the array branch returns before this block, so arrays with null-prototype elements are only affected via recursion (as tested).

Extended reasoning...

Overview

Adds a null-prototype check to Bun__deepEquals<isStrict=true> in src/jsc/bindings/bindings.cpp so toStrictEqual and Bun.deepEquals(a, b, true) distinguish Object.create(null) from {}, matching jest/vitest. The fixing hunk is 10 lines: after the existing calculatedClassName comparison, read both [[Prototype]]s and return false when exactly one is null. Two comment lines were reworded, two docs pages gained one example each, an existing it.failing test was un-marked and extended, and new tests were added to expect.test.js and deep-equals.test.ts.

Security risks

None. This is a pure comparison-semantics change in the test runner / Bun.deepEquals utility. No parsing of untrusted input, no auth/crypto/permissions.

Level of scrutiny

Medium — a small native change in a heavily-templated equality function that backs multiple entry points. I traced all six deepEqualsWrapperImpl call sites to confirm the new block compiles into exactly the two intended instantiations (<true, false, false> and <true, true, false>): loose mode is gated out by isStrict, the node paths by !checkPrototypes (added after my earlier review), and the skip-proto variant by !skipPrototypeIdentity. Exception checks after each getPrototype are present, and the Proxy test with a throwing getPrototypeOf trap exercises them from both sides.

Other factors

My previous review flagged that the node entry point would invoke the Proxy getPrototypeOf trap twice; that was fixed in 9c27643 by wrapping the new lookups in if constexpr (!checkPrototypes), and I verified the current diff reflects that. The comment-cop bot's paragraph-comment complaints were addressed in fc00b84/6afda93. The PR description documents that the new tests fail on Bun 1.4.0 and pass with the change, and that test/js/node/assert/ and other toStrictEqual users in the repo were checked for regressions. Test coverage is thorough: both operand orders, nested in object properties and array elements, two null-prototype objects still equal, Object.groupBy, Proxies (default forwarding and throwing trap), and toEqual staying unchanged for every case.

…quals tests

claude

LGTM — the redundant getPrototype on the node path from my earlier review is addressed, and the comment-cop feedback is resolved.

What was reviewed:

Traced all six deepEqualsWrapperImpl instantiations against the isStrict && !skipPrototypeIdentity && !checkPrototypes guard: only Bun.deepEquals(_, _, true) and toStrictEqual reach the new code; the node and skipPrototype paths compile it out.
Checked exception handling — both new getPrototype calls have RETURN_IF_EXCEPTION and match the pattern at lines 822-825.
Tests cover both operand orders, nesting, arrays, two null-protos still equal, Proxy trap semantics, and a guard that util.isDeepStrictEqual(_, _, true) stays prototype-blind.

Extended reasoning...

Overview

Adds a null-prototype vs Object.prototype distinction to strict Bun.deepEquals and expect().toStrictEqual(), matching jest/vitest. The C++ change is 8 lines in bindings.cpp:959-967: inside the existing isStrict && !skipPrototypeIdentity class-name check, and further gated by if constexpr (!checkPrototypes), it reads both [[Prototype]]s and returns false when exactly one is null. Two pre-existing comments describing the code as "prototype-blind" are reworded. Two docs pages get a one-line addition to the strict-mode difference list. An existing it.failing test is un-marked and extended, and comprehensive new tests are added in deep-equals.test.ts and expect.test.js.

Security risks

None. This is a comparison helper in the test runner / Bun namespace; no untrusted-input parsing, no auth/crypto/permissions.

Level of scrutiny

Medium. It's a behavioral change to a widely-used matcher, but it's a compat fix moving Bun toward the jest/vitest reference behavior, and the author ran the repo's toStrictEqual users to confirm nothing else relied on the old behavior. The native change follows the identical getPrototype + RETURN_IF_EXCEPTION pattern used 30 lines above at 822-825, so the exception-safety review reduces to pattern-matching. I traced each of the six entry points (deepEquals, jestDeepEquals, strictDeepEquals, jestStrictDeepEquals, Bun__deepEqualsNodeStrict, Bun__deepEqualsNodeStrictSkipProto) through the two nested if constexpr guards and confirmed only the two intended ones (<true,*,false,false>) reach the new code; the node path (checkPrototypes=true) and skipPrototype path (skipPrototypeIdentity=true) compile it out entirely.

Other factors

My earlier inline review flagged a redundant getPrototype call on the node entry point (double trap invocation); commit 9c27643 wrapped the new lookups in if constexpr (!checkPrototypes), which resolves it — the node path now compiles to exactly what it was before. The comment-cop bot's length complaints were addressed in 6afda93. All review threads are resolved. Test coverage is thorough: symmetric operand orders, nested-in-object, nested-in-array, two null-protos still deep-equal (and still differ on values / undefined props), Object.groupBy, Proxy pass-through and Proxy trap-override, and a regression guard on util.isDeepStrictEqual(_, _, true). No CODEOWNERS on bindings.cpp. The bug-hunting system found nothing.

github-actions Bot added the claude label Aug 12, 2026

robobun mentioned this pull request Aug 12, 2026

Bun.deepEquals: fix four false-positive equality classes #32872

Open

claude Bot reviewed Aug 12, 2026

View reviewed changes